Skip to content

fix(core): validate sample_count is 1 in Queue::write_texture#7984

Closed
ErichDonGubler wants to merge 1 commit intogfx-rs:trunkfrom
erichdongubler-mozilla:write_texture-sample-count-1
Closed

fix(core): validate sample_count is 1 in Queue::write_texture#7984
ErichDonGubler wants to merge 1 commit intogfx-rs:trunkfrom
erichdongubler-mozilla:write_texture-sample-count-1

Conversation

@ErichDonGubler
Copy link
Member

This is a missing piece of the "validating texture buffer copy" routine, which is currently not outlined in the same way as the WebGPU spec.

It's almost certainly better to refactor writeTexture and other copy commands so that we can capture the routine and properly share it between different standard operations. This commit leaves this as future work, only fixing the missing validation in writeTexture.

Connections

Squash or Rebase?

Squash.

Checklist

  • If this contains user-facing changes, add a CHANGELOG.md entry.

@ErichDonGubler ErichDonGubler requested review from a team and crowlKats as code owners July 21, 2025 20:59
This is a missing piece of the ["validating texture buffer copy"
routine](https://www.w3.org/TR/webgpu/#abstract-opdef-validating-texture-buffer-copy),
which is currently not outlined in the same way as the WebGPU spec.

It's almost certainly better to refactor `writeTexture` and other copy
commands so that we can capture the routine and properly share it
between different standard operations. This commit leaves this as future
work, only fixing the missing validation in `writeTexture.`
@ErichDonGubler ErichDonGubler force-pushed the write_texture-sample-count-1 branch from d19f441 to d893b71 Compare July 21, 2025 21:00
@ErichDonGubler
Copy link
Member Author

A Rust MRE, if anyone is interested:

//! A Rust-y version of
//! [`webgpu:api,validation,queue,writeTexture:sample_count:*`](https://github.com/gpuweb/cts/blob/26530c7da1c48ea5d0bf4c5da83b15b996eb522f/src/webgpu/api/validation/queue/writeTexture.spec.ts#L60-L85).

use pollster::FutureExt as _;

fn main() {
    env_logger::init();

    let instance = wgpu::Instance::new(&wgpu::InstanceDescriptor::from_env_or_default());
    let adapter = instance
        .request_adapter(&Default::default())
        .block_on()
        .unwrap();
    let (device, queue) = adapter
        .request_device(&Default::default())
        .block_on()
        .unwrap();

    for sample_count in [1, 4] {
        let texture = device.create_texture(&wgpu::TextureDescriptor {
            label: None,
            size: wgpu::Extent3d {
                width: 16,
                height: 16,
                depth_or_array_layers: 1,
            },
            mip_level_count: 1,
            sample_count,
            dimension: wgpu::TextureDimension::D2,
            format: wgpu::TextureFormat::Bgra8Unorm,
            usage: wgpu::TextureUsages::COPY_DST | wgpu::TextureUsages::RENDER_ATTACHMENT,
            view_formats: &[],
        });

        let data = [0u8; 16];
        let size = wgpu::Extent3d {
            width: 1,
            height: 1,
            depth_or_array_layers: 1,
        };

        let is_valid = sample_count == 1;

        device.push_error_scope(wgpu::ErrorFilter::Validation);

        queue.write_texture(
            wgpu::TexelCopyTextureInfoBase {
                texture: &texture,
                mip_level: 0,
                origin: Default::default(),
                aspect: Default::default(),
            },
            &data,
            Default::default(),
            size,
        );

        match (is_valid, device.pop_error_scope().block_on()) {
            (true, None) | (false, Some(..)) => (),
            (expected_valid, err_opt) => panic!(
                concat!(
                    "expected valid = {} for `sample_count` = {}, ",
                    "but error scope yielded {:?}"
                ),
                expected_valid, sample_count, err_opt
            ),
        }
    }
}

@cwfitzgerald
Copy link
Member

cwfitzgerald commented Jul 21, 2025

Couldyoumaybeaddthatasavalidationtestmaybe?

@andyleiserson
Copy link
Contributor

This fix is also included in #7948.

@ErichDonGubler
Copy link
Member Author

ErichDonGubler commented Jul 22, 2025

@cwfitzgerald:

Couldyoumaybeaddthatasavalidationtestmaybe?

Does…the relevant CTS test being added to CI not count? 🤔

@andyleiserson
Copy link
Contributor

I didn't add the same CTS test (webgpu:api,validation,queue,writeTexture:sample_count:*) in the other PR, so it would be worth adding that. The diff hunk in queue.rs isn't needed.

@ErichDonGubler
Copy link
Member Author

I see that all parts of this PR have been subsumed into #7948 (nice number, BTW!), which has been merged into trunk. Abandoning.

@ErichDonGubler ErichDonGubler deleted the write_texture-sample-count-1 branch July 23, 2025 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants